33292 replace AspectMock from BaseGenerateCommandTest #840
Conversation
|
@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework |
|
@SilinMykola |
|
@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework |
| $testObjectHandler = TestObjectHandler::getInstance(); | ||
| $reflection = new \ReflectionClass(TestObjectHandler::class); | ||
| $reflectionMethod = $reflection->getMethod('initTestData'); | ||
| $reflectionMethod->setAccessible(true); | ||
| $output = $reflectionMethod->invoke($testObjectHandler); | ||
| $this->assertEquals('', $output); | ||
|
|
There was a problem hiding this comment.
This code does not seem to be necessary since we are not stubbing initTestData?
This is not a usual way for unit tests, but it does work for eliminating the usage of AspectMock with no framework refactoring. We can use this solution.
There was a problem hiding this comment.
@jilu1 Hello! I removed this code.
| $suiteObjectHandler = SuiteObjectHandler::getInstance(); | ||
| $reflection = new \ReflectionClass(SuiteObjectHandler::class); | ||
| $reflectionMethod = $reflection->getMethod('initSuiteData'); | ||
| $reflectionMethod->setAccessible(true); | ||
| $output = $reflectionMethod->invoke($suiteObjectHandler); | ||
| $this->assertEquals('', $output); | ||
|
|
There was a problem hiding this comment.
Similarly, is this code necessary since we are not stubbing initSuiteData?
| public function tearDown(): void | ||
| { | ||
| AspectMock::clean(); | ||
| } | ||
|
|
There was a problem hiding this comment.
As a precaution, can we set tests and suites to [] forTestObjectHandler and SuiteObjectHandler in tearDown()?
There was a problem hiding this comment.
@jilu1 I changed tearDown method and set [] for TestObjectHandler and SuiteObjectHandler
|
@SilinMykola thanks for the pull request! Would you be able to respond to the code review comments? |
|
@sivaschenko yes, I'll try to change code after review. I need some time for this. |
|
Hello, @jilu1 I have a little bit helped for @SilinMykola with the aspect mock replacement here to match the original behaviour (+ with the implementation of what you asked for). Please, proceed with the code review. cc: @SilinMykola, @sivaschenko |
There was a problem hiding this comment.
Hello @SilinMykola @bohdan-harniuk
Now changes look well for me.
Thank you for your contribution.
Hello @jilu1 could you please recheck
Thank you in advance.
|
@magento import pull request to https://github.com/magento-commerce/magento2-functional-testing-framework |
|
@jilu1 the pull request successfully imported. |
bohdan-harniuk
added
Partner: Atwix
partners-contribution
|
@bohdan-harniuk |
|
Hello, @jilu1 This task was finished without creating new singletons, so it is not a blocker. We can go further here. Thanks, Bohdan |
SilinMykola
deleted the
33292-replace-AspectMock-for-BaseGenerateCommandTest
branch
Description
Eliminate AspectMock usage from
dev/tests/unit/Magento/FunctionalTestFramework/Console/BaseGenerateCommandTest.phpFixed Issues (if relevant)
Contribution checklist